feat(PrizePool): redesign table via Table2 with collapse rework#7688
feat(PrizePool): redesign table via Table2 with collapse rework#7688Eetwalt wants to merge 35 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR modernizes prize pool table rendering by migrating from the legacy table widget to Table2, updating column order/alignment, and replacing the old per-row collapse mechanism with a general-collapsible footer toggle.
Changes:
- Migrate
Module:PrizePool/*table output to Table2 (header/body/footer) with updated column ordering and alignment. - Rework collapse behavior to use
general-collapsible+ CSS-hidden rows instead of legacy injected toggle rows (and skip the legacy JS toggle for the new wrapper). - Add top-3 placement badge/tint styling and switch place display from ordinals to numeric ranges.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| stylesheets/commons/Prizepooltable.scss | Restyles prize pool tables for Table2 structure, new top-3 visuals, and wrapper-based collapse behavior. |
| lua/wikis/commons/Widget/Table2/CellHeader.lua | Ensures rowspan/colspan attributes are emitted even without column definitions. |
| lua/wikis/commons/Widget/Table2/Cell.lua | Ensures rowspan/colspan attributes are emitted even without column definitions. |
| lua/wikis/commons/PrizePool/Placement.lua | Switches place display to numeric/ranges and adds badge class helper for top-3. |
| lua/wikis/commons/PrizePool/Base.lua | Core migration to Table2, new collapse toggle footer, and new prize-cell matrix merging logic. |
| lua/wikis/commons/PrizePool/Award.lua | Updates award table to Table2 and adopts new collapse label mechanism. |
| lua/wikis/commons/PrizePool.lua | Updates placement prize pool rendering (badges, collapse labels) and removes legacy toggle logic. |
| lua/wikis/commons/Placement.lua | Removes the 4th-place background class mapping. |
| lua/spec/prize_pool_spec.lua | Updates test expectations to match new prize column sort order. |
| javascript/commons/Prizepooltable.js | Skips legacy toggle injection for the redesigned Table2 prize pool wrapper to avoid duplicate toggles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| local cell = reward and prizeTypeData.rowDisplay(prize.data, reward) or TableCell{} | ||
|
|
||
| previousPlacement = placement | ||
| end | ||
| local lastCellOfType = previousOfPrizeType[prize.type] | ||
| if lastCellOfType and prizeTypeData.mergeDisplayColumns then | ||
| if Table.isNotEmpty(lastCellOfType.props.children) and Table.isNotEmpty(cell.props.children) then |
There was a problem hiding this comment.
cell.props.children is never nil but empty table, so Table.isNotEmpty handles that.
There was a problem hiding this comment.
maybe use Logic.isNotEmpty for more flexibility just in case?
ElectricalBoy
left a comment
There was a problem hiding this comment.
from a quick glance on phone
There was a problem hiding this comment.
I am somewhat confident that we cannot kick the existing styles (blame smash)
There was a problem hiding this comment.
Ah.. Probably so. Reverted with some comments
| ---@protected | ||
| --- Whether a placement's rows are hidden behind the collapse toggle. | ||
| --- Child classes override this; the default keeps every row visible. | ||
| ---@param placement BasePlacement | ||
| ---@return boolean | ||
| function BasePrizePool:applyCutAfter(placement) | ||
| error('Function applyCutAfter needs to be implemented by a child class of "Module:PrizePool/Base"') | ||
| return false | ||
| end | ||
|
|
||
| ---@protected | ||
| ---@param placement BasePlacement? | ||
| ---@param nextPlacement BasePlacement | ||
| ---@param row WidgetTableRow | ||
| function BasePrizePool:applyToggleExpand(placement, nextPlacement, row) | ||
| error('Function applyToggleExpand needs to be implemented by a child class of "Module:PrizePool/Base"') | ||
| ---Open/close labels for the collapse toggle. Child classes override. | ||
| ---@return {opentext: string, closetext: string}? | ||
| function BasePrizePool:_collapseText() | ||
| return nil | ||
| end |
There was a problem hiding this comment.
should keep @protected annotations
| ---Returns the placement-badge color class for top-3 placements, else nil. | ||
| ---Colored by the top of the range (placeStart). | ||
| ---@return string? | ||
| function Placement:getBadgeClass() | ||
| if self:hasSpecialStatus() then | ||
| return | ||
| end | ||
| local badges = {[1] = 'placement-1', [2] = 'placement-2', [3] = 'placement-3'} | ||
| return badges[self.placeStart] | ||
| end |
There was a problem hiding this comment.
should make use of Module:Placement
| &::-webkit-scrollbar { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
somewhat redundant as we already have scrollbar-width: none (tbf scrollbar-width itself is baseline newly available)
maybe hide this behind @supports not (scrollbar-width: none)?
| // The theme wrapper + `.table2__table` qualifier raise specificity above the | ||
| // Table2 zebra rule so the tint wins regardless of load order; `:hover` keeps it. | ||
| // `border-bottom: 0` drops the legacy per-place coloured row border. | ||
| .theme--light &.prizepooltable-placement.table2__table, | ||
| .theme--dark &.prizepooltable-placement.table2__table { |
There was a problem hiding this comment.
maybe lower table2 zebra rules' specificities with :where() instead?
not a fan of having a lot of table2-specific selectors here
| children = {tostring(OpponentDisplay.BlockOpponent{ | ||
| opponent = opponent.opponentData, | ||
| showPlayerTeam = true, | ||
| })}, |
There was a problem hiding this comment.
tostring is redundant
| children = { | ||
| Span{ | ||
| classes = {'general-collapsible-expand-button'}, | ||
| children = {Span{children = {collapseText.opentext, ' ', IconFa{iconName = 'expand'}}}}, | ||
| }, | ||
| css = {width = 'max-content'}, | ||
| columns = headerRow:getCellCount(), | ||
| children = WidgetUtil.collect(headerRow, unpack(self:_buildRows())) | ||
| }}, | ||
| Span{ | ||
| classes = {'general-collapsible-collapse-button'}, | ||
| children = {Span{children = {collapseText.closetext, ' ', IconFa{iconName = 'collapse'}}}}, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
could (should?) use Module:Widget/GeneralCollapsible/ChevronToggle
There was a problem hiding this comment.
Needs the text so not as is, but I extended the Chevron to accept text and size inputs to make it work. Might be useful down the line in other places
There was a problem hiding this comment.
what if you use data-collapsible-click-region=true?
| return TableCell{children = {table.concat(headerDisplay)}} | ||
| return TableCellHeader{children = {table.concat(headerDisplay)}, align = 'right'} |
There was a problem hiding this comment.
table.concat is redundant
| ---@param placement BasePlacement | ||
| ---@param opponent table | ||
| ---@return Renderable[] | ||
| function BasePrizePool:_opponentPrizeCells(placement, opponent) |
There was a problem hiding this comment.
| ---@param placement BasePlacement | |
| ---@param opponent table | |
| ---@return Renderable[] | |
| function BasePrizePool:_opponentPrizeCells(placement, opponent) | |
| ---@private | |
| ---@param placement BasePlacement | |
| ---@param opponent BasePlacementOpponent | |
| ---@return Renderable[] | |
| function BasePrizePool:_opponentPrizeCells(placement, opponent) |
There was a problem hiding this comment.
extract column definitions to separate field in prize types table instead of adding all the aligns in cells
same for opponent column too
|
image export config for prize pools should be updated as well |
This can be in a separate PR imo |
there already is #7687 |
there's no way that there won't be a merge conflict |
| &.prizepooltable-placement { | ||
| tr.table2__row--body.background-color-first-place { | ||
| border-bottom: 0; | ||
|
|
||
| &, | ||
| &:hover { | ||
| background-color: var( --prizepool-place-1-tint ); | ||
| } | ||
|
|
||
| .prizepooltable-place::before { | ||
| background-color: var( --prizepool-place-1-accent ); | ||
| } | ||
| } | ||
|
|
||
| tr.table2__row--body.background-color-second-place { | ||
| border-bottom: 0; | ||
|
|
||
| &, | ||
| &:hover { | ||
| background-color: var( --prizepool-place-2-tint ); | ||
| } | ||
|
|
||
| .prizepooltable-place::before { | ||
| background-color: var( --prizepool-place-2-accent ); | ||
| } | ||
| } | ||
|
|
||
| tr.table2__row--body.background-color-third-place { | ||
| border-bottom: 0; | ||
|
|
||
| &, | ||
| &:hover { | ||
| background-color: var( --prizepool-place-3-tint ); | ||
| } | ||
|
|
||
| .prizepooltable-place::before { | ||
| background-color: var( --prizepool-place-3-accent ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I think it is better to have a separate color for hovering to keep consistency with other rows (like how standings table handles hovering)
Lua-Modules/stylesheets/commons/Standings.scss
Lines 65 to 75 in 10e8def
| &.prizepooltable-placement .prizepooltable-place { | ||
| text-align: center; | ||
| font-weight: bold; | ||
| color: var( --clr-secondary-25 ); | ||
|
|
||
| // Override the legacy `… > td.prizepooltable-place` metal fill so the row | ||
| // tint + accent bar show through on the redesigned table instead. | ||
| background-color: transparent; | ||
|
|
||
| .theme--dark & { | ||
| color: var( --clr-secondary-100 ); | ||
| } | ||
| } | ||
|
|
||
| // Top-3 rows: left accent bar (coloured, with the row tint, in the theme block below). | ||
| &.prizepooltable-placement { | ||
| tr.table2__row--body.background-color-first-place, | ||
| tr.table2__row--body.background-color-second-place, | ||
| tr.table2__row--body.background-color-third-place { | ||
| .prizepooltable-place { | ||
| position: relative; | ||
|
|
||
| &::before { | ||
| content: ""; | ||
| position: absolute; | ||
| inset: 0 auto 0 0; | ||
| width: $prizepool-accent-width; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
| &.prizepooltable-placement .prizepooltable-place { | |
| text-align: center; | |
| font-weight: bold; | |
| color: var( --clr-secondary-25 ); | |
| // Override the legacy `… > td.prizepooltable-place` metal fill so the row | |
| // tint + accent bar show through on the redesigned table instead. | |
| background-color: transparent; | |
| .theme--dark & { | |
| color: var( --clr-secondary-100 ); | |
| } | |
| } | |
| // Top-3 rows: left accent bar (coloured, with the row tint, in the theme block below). | |
| &.prizepooltable-placement { | |
| tr.table2__row--body.background-color-first-place, | |
| tr.table2__row--body.background-color-second-place, | |
| tr.table2__row--body.background-color-third-place { | |
| .prizepooltable-place { | |
| position: relative; | |
| &::before { | |
| content: ""; | |
| position: absolute; | |
| inset: 0 auto 0 0; | |
| width: $prizepool-accent-width; | |
| } | |
| } | |
| } | |
| } | |
| &.prizepooltable-placement { | |
| .prizepooltable-place { | |
| text-align: center; | |
| font-weight: bold; | |
| color: var( --clr-secondary-25 ); | |
| // Override the legacy `… > td.prizepooltable-place` metal fill so the row | |
| // tint + accent bar show through on the redesigned table instead. | |
| background-color: transparent; | |
| .theme--dark & { | |
| color: var( --clr-secondary-100 ); | |
| } | |
| } | |
| // Top-3 rows: left accent bar (coloured, with the row tint, in the theme block below). | |
| tr.table2__row--body.background-color-first-place, | |
| tr.table2__row--body.background-color-second-place, | |
| tr.table2__row--body.background-color-third-place { | |
| .prizepooltable-place { | |
| position: relative; | |
| &::before { | |
| content: ""; | |
| position: absolute; | |
| inset: 0 auto 0 0; | |
| width: $prizepool-accent-width; | |
| } | |
| } | |
| } | |
| } |
| props = props or {} | ||
| local size = props.size or 'xs' |
Summary
Widget/TabletoWidget/Table2(header/body/footer sections, declarativerowspan/colspan)general-collapsibletoggle rendered in the footer, replacing the per-rowapplyToggleExpandmachinery; child classes now only supply open/close label textArray.groupAdjacentBy/Table.deepEquals, extracted into_opponentPrizeCells1,1-2) instead of ordinals, with colored gold/silver/bronze badges and pale row tints for top-3.prizepool-table-wrapperrowspan/colspanfrom Table2Cell/CellHeadereven without column definitionsHow did you test this change?
dev + devtools